Skip to content

fix(sdk-lib-mpc): authenticate signatureR in DKLS DSG round 4 messages#8470

Merged
mrdanish26 merged 2 commits intomasterfrom
fix/wal-376-dkls-dsg-signaturer-authentication
Apr 15, 2026
Merged

fix(sdk-lib-mpc): authenticate signatureR in DKLS DSG round 4 messages#8470
mrdanish26 merged 2 commits intomasterfrom
fix/wal-376-dkls-dsg-signaturer-authentication

Conversation

@mrdanish26
Copy link
Copy Markdown
Contributor

@mrdanish26 mrdanish26 commented Apr 9, 2026

Summary

DKLS DSG round 4 — authenticate signatureR

  • Sign signatureR bytes with the party GPG key in encryptAndAuthOutgoingMessages() — previously a hardcoded empty signature left the ECDSA nonce commitment R unauthenticated (F-04, severity: HIGH).
  • Verify signatureR in decryptAndVerifyIncomingMessages() before returning it to callers.
  • Transmit signatureR and signatureRSignature on round-3 / round-4 plumbing so the server can authenticate R before combinePartialSignatures() (aligned with @bitgo/public-types).

Types & SDK wiring

  • Bump @bitgo/public-types (e.g. 5.92.0) so msg4 includes signatureRSignature and the flow is typed end-to-end (reduces need for ad-hoc casts in tests). PR
  • sdk-core: require signatureR.message / signatureR.signature where applicable; set signatureR + signatureRSignature on msg4 for MPCv2SignatureShareRound3Input.
  • express tests (externalSign): pass signatureR auth when both signatureR and signatureRSignature are present; soft downgrade when not.

Express build (TS7056)

  • Annotate heavy httpRoute exports as HttpRoute<'post'> (PostSendMany, PostSendCoins, PostWalletSignTx, PostWalletTxSignTSS) so .d.ts declaration emit stays under TypeScript’s serialization limit; update typedRoutes/api/index.ts notes accordingly.

Test plan

  • sdk-lib-mpc unit tests (sign/verify round-trip, tampered R, wrong key, no-signatureR / soft-downgrade cases as applicable)
  • Relevant sdk-core / bitgo tests for MPCv2 / external-sign paths
  • @bitgo/express builds (tsc; TS7056 resolved for the routes above)

Ticket: WAL-376

@linear
Copy link
Copy Markdown

linear bot commented Apr 9, 2026

@mrdanish26 mrdanish26 marked this pull request as ready for review April 10, 2026 20:14
@mrdanish26 mrdanish26 requested review from a team as code owners April 10, 2026 20:14
Comment thread modules/express/test/unit/clientRoutes/externalSign.ts Outdated
@sachushaji
Copy link
Copy Markdown
Contributor

@claude

@mrdanish26 mrdanish26 requested review from a team as code owners April 14, 2026 20:31
@mrdanish26 mrdanish26 marked this pull request as draft April 14, 2026 20:32
@mrdanish26 mrdanish26 force-pushed the fix/wal-376-dkls-dsg-signaturer-authentication branch 3 times, most recently from 09a86ef to f135d5e Compare April 14, 2026 22:45
Annotate PostSendMany, PostSendCoins, PostWalletSignTx, and PostWalletTxSignTSS
as HttpRoute<'post'> so declaration emit stays within TS limits.

WAL-376
@mrdanish26 mrdanish26 force-pushed the fix/wal-376-dkls-dsg-signaturer-authentication branch from f135d5e to 905a4ae Compare April 14, 2026 23:35
@mrdanish26 mrdanish26 marked this pull request as ready for review April 15, 2026 00:06
@jamesmili jamesmili removed the request for review from a team April 15, 2026 17:31
Copy link
Copy Markdown
Contributor

@alia-bitgo alia-bitgo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm- not sure if ethalt should own modules/sdk-core/src/bitgo/tss/ecdsa/ecdsaMPCv2.ts though

Copy link
Copy Markdown

@bhargavirao24 bhargavirao24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed and approved.
These NX packages are just platform binaries already on master, they are not new in this PR. SafeChain is blocking because they were published 4 days ago. I reviewed the dependency, security scores are fine with no alerts flagged. Maintainer hasn't changed between versions and publishes are via GitHub Actions CI. Approved the PR.

@mrdanish26 mrdanish26 merged commit e5fa843 into master Apr 15, 2026
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants